Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

php spark serve #996

Merged
merged 2 commits into from
Apr 25, 2018
Merged

php spark serve #996

merged 2 commits into from
Apr 25, 2018

Conversation

natanfelles
Copy link
Contributor

Move the development server to a spark command.

Description:
   Launchs the CodeIgniter PHP-Development Server.

Usage:
   serve

Options:
   -php       The PHP Binary [default: "PHP_BINARY"]
   -host      The HTTP Host [default: "localhost"]
   -port      The HTTP Host Port [default: "8080"]

@deathart
Copy link
Contributor

deathart commented Apr 22, 2018

I'm enjoying your Pull Request (which I think is perfect).

To say that the last line of the serve file I must modify it to

Before :

passthru("{$php} -S {$host}:{$port} -t public/ rewrite.php");

After :

passthru("{$php} -S {$host}:{$port} -t public/ .../rewrite.php");

I have to add ../ before rewrite.php

(Windows)

@lonnieezell
Copy link
Member

@natanfelles What benefit do you see by putting it behind the spark command? I debated doing it both ways when I created it, but thought it made for unnecessary overhead with no benefit at the time. Interested to hear your reasoning.

@natanfelles
Copy link
Contributor Author

@deathart , in fact this PR should not be so perfect... But the intention is to try to improve.

@lonnieezell , the intent is to make this command more visible when the spark is executed. In addition to decreasing the number of executables outside the system and application folders.

I added the --php option thinking about the possibility of running spark by ./spark or with the spark command itself in a global context.

I do not know if application/Config/Rewrite.php is the best place to put the server router, but I thought the user might have changed the path of the Front Controller or want to pass custom $_SERVER variables, so it stayed there.

@jim-parry
Copy link
Contributor

Are you referring to a "Laravel spark" (scaffolding?), or the "spark" microframework, or the long-gone Ci "spark"? I could be out of it, but I don't recall this coming up recently in a CI context.

@natanfelles natanfelles changed the title php spark serve [proposed] php spark serve Apr 22, 2018
@natanfelles
Copy link
Contributor Author

I could refer to "Symfony Console spark", but no. Just a PR, yous decide.

@jim-parry
Copy link
Contributor

Ok, I can't even find a reference to "Symfony console spark". It sounds like that could be another interpretation of the "verb/command". I see that this came up on the forum, and there was a lot of support for the term, which is unfortunate given the other possible interpretations. I didn't even think to refer to the forum until I remembered the thread on the CLI tool name :-/

@lonnieezell
Copy link
Member

@natanfelles That's a fair point. I'm fine with moving this to a command, then. However - I'm really not a fan of having the rewrite class in Config. It's not a configuration file. It is needed to make the server work as expected, though.

I propose:

  • New system\Commands\Server directory.
  • Put both serve and rewrite in that folder.

Then I'll accept.

@lonnieezell lonnieezell merged commit 162dfa6 into codeigniter4:develop Apr 25, 2018
ytetsuro pushed a commit to ytetsuro/codeigniter-composer-installer that referenced this pull request Jul 20, 2018
Because the file composition has changed by the next PR of Code Igniter 4,
editing of a nonexistent file and deletion of the code that is about to be copied.

codeigniter4/CodeIgniter4#996
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants